From 34ace11060f1c410ba49dcbbdf288d7a6fe7a92b Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 27 Oct 2014 15:13:37 +0100 Subject: [PATCH] Build command output now being checked and written to filesystem --- src/cargo/ops/cargo_rustc/context.rs | 10 +- src/cargo/ops/cargo_rustc/layout.rs | 39 +++++ src/cargo/ops/cargo_rustc/mod.rs | 203 ++++++++++++++++++----- src/cargo/ops/mod.rs | 2 +- tests/test_cargo_compile_custom_build.rs | 33 +++- 5 files changed, 243 insertions(+), 44 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 7192aab7d..fdc0b988b 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -6,7 +6,7 @@ use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target}; use util::{mod, CargoResult, ChainError, internal, Config, profile}; use util::human; -use super::{Kind, KindPlugin, KindTarget, Compilation}; +use super::{Kind, KindForHost, KindTarget, Compilation}; use super::layout::{Layout, LayoutProxy}; #[deriving(Show)] @@ -168,10 +168,10 @@ impl<'a, 'b: 'a> Context<'a, 'b> { pub fn layout(&self, pkg: &Package, kind: Kind) -> LayoutProxy { let primary = pkg.get_package_id() == self.resolve.root(); match kind { - KindPlugin => LayoutProxy::new(&self.host, primary), + KindForHost => LayoutProxy::new(&self.host, primary), KindTarget => LayoutProxy::new(self.target.as_ref() .unwrap_or(&self.host), - primary) + primary), } } @@ -180,7 +180,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> { /// If `plugin` is true, the pair corresponds to the host platform, /// otherwise it corresponds to the target platform. fn dylib(&self, kind: Kind) -> CargoResult<(&str, &str)> { - let (triple, pair) = if kind == KindPlugin { + let (triple, pair) = if kind == KindForHost { (self.config.rustc_host(), &self.host_dylib) } else { (self.target_triple.as_slice(), &self.target_dylib) @@ -208,7 +208,7 @@ impl<'a, 'b: 'a> Context<'a, 'b> { } else { if target.is_dylib() { let plugin = target.get_profile().is_for_host(); - let kind = if plugin {KindPlugin} else {KindTarget}; + let kind = if plugin {KindForHost} else {KindTarget}; let (prefix, suffix) = try!(self.dylib(kind)); ret.push(format!("{}{}{}", prefix, stem, suffix)); } diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index b2fd7bf88..65036d1c8 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -16,6 +16,20 @@ //! //! # This is the location at which the output of all custom build //! # commands are rooted +//! build/ +//! +//! # Each package gets its own directory where its build script and +//! # script output are placed +//! $pkg1/ +//! $pkg2/ +//! $pkg3/ +//! +//! # Each directory package has a `out` directory where output +//! # is placed. +//! out/ +//! +//! # This is the location at which the output of all old custom build +//! # commands are rooted //! native/ //! //! # Each package gets its own directory for where its output is @@ -46,6 +60,7 @@ //! //! # Same as the two above old directories //! old-native/ +//! old-build/ //! old-fingerprint/ //! old-examples/ //! ``` @@ -60,12 +75,14 @@ pub struct Layout { root: Path, deps: Path, native: Path, + build: Path, fingerprint: Path, examples: Path, old_deps: Path, old_root: Path, old_native: Path, + old_build: Path, old_fingerprint: Path, old_examples: Path, } @@ -93,11 +110,13 @@ impl Layout { Layout { deps: root.join("deps"), native: root.join("native"), + build: root.join("build"), fingerprint: root.join(".fingerprint"), examples: root.join("examples"), old_deps: root.join("old-deps"), old_root: root.join("old-root"), old_native: root.join("old-native"), + old_build: root.join("old-build"), old_fingerprint: root.join("old-fingerprint"), old_examples: root.join("old-examples"), root: root, @@ -153,6 +172,14 @@ impl Layout { self.fingerprint.join(self.pkg_dir(package)) } + pub fn build(&self, package: &Package) -> Path { + self.build.join(self.pkg_dir(package)) + } + + pub fn build_out(&self, package: &Package) -> Path { + self.build(package).join("out") + } + pub fn old_dest<'a>(&'a self) -> &'a Path { &self.old_root } pub fn old_deps<'a>(&'a self) -> &'a Path { &self.old_deps } pub fn old_examples<'a>(&'a self) -> &'a Path { &self.old_examples } @@ -163,6 +190,10 @@ impl Layout { self.old_fingerprint.join(self.pkg_dir(package)) } + pub fn old_build(&self, package: &Package) -> Path { + self.old_build.join(self.pkg_dir(package)) + } + fn pkg_dir(&self, pkg: &Package) -> String { format!("{}-{}", pkg.get_name(), short_hash(pkg.get_package_id())) } @@ -195,6 +226,10 @@ impl<'a> LayoutProxy<'a> { pub fn native(&self, pkg: &Package) -> Path { self.root.native(pkg) } + pub fn build(&self, pkg: &Package) -> Path { self.root.build(pkg) } + + pub fn build_out(&self, pkg: &Package) -> Path { self.root.build_out(pkg) } + pub fn old_root(&self) -> &'a Path { if self.primary {self.root.old_dest()} else {self.root.old_deps()} } @@ -205,5 +240,9 @@ impl<'a> LayoutProxy<'a> { self.root.old_native(pkg) } + pub fn old_build(&self, pkg: &Package) -> Path { + self.root.old_build(pkg) + } + pub fn proxy(&self) -> &'a Layout { self.root } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 774675405..84a6979c8 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; use std::dynamic_lib::DynamicLibrary; -use std::io::{fs, USER_RWX}; +use std::io::{fs, BufReader, USER_RWX}; use std::io::fs::PathExtensions; use core::{SourceMap, Package, PackageId, PackageSet, Target, Resolve}; @@ -25,7 +25,7 @@ mod job_queue; mod layout; #[deriving(PartialEq, Eq)] -pub enum Kind { KindPlugin, KindTarget } +pub enum Kind { KindForHost, KindTarget } /// Run `rustc` to figure out what its current version string is. /// @@ -148,7 +148,7 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, let (target1, target2) = fingerprint::prepare_init(cx, pkg, KindTarget); let mut init = vec![(Job::new(target1, target2, String::new()), Fresh)]; if cx.config.target().is_some() { - let (plugin1, plugin2) = fingerprint::prepare_init(cx, pkg, KindPlugin); + let (plugin1, plugin2) = fingerprint::prepare_init(cx, pkg, KindForHost); init.push((Job::new(plugin1, plugin2, String::new()), Fresh)); } jobs.enqueue(pkg, jq::StageStart, init); @@ -171,10 +171,40 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, if target.get_profile().is_custom_build() { for &(ref mut work, _, _) in rustc.iter_mut() { use std::mem; - let execute_cmd = try!(prepare_execute_custom_build(pkg, root_pkg, + + let (old_build, script_output) = { + let layout = cx.layout(pkg, KindForHost); + let old_build = layout.proxy().old_build(pkg); + let script_output = layout.build(pkg); + (old_build, script_output) + }; + + let execute_cmd = try!(prepare_execute_custom_build(pkg, + root_pkg, target, cx)); + + // building a `Work` that creates the directory where the compiled script + // must be placed + let create_directory = proc() { + if old_build.exists() { + fs::rename(&old_build, &script_output) + } else { + fs::mkdir_recursive(&script_output, USER_RWX) + }.chain_error(|| { + internal("failed to create script output directory for build command") + }) + }; + + // replacing the simple rustc compilation by three steps: + // 1 - create the output directory + // 2 - call rustc + // 3 - execute the command let rustc_cmd = mem::replace(work, proc() Ok(())); - let replacement = proc() { try!(rustc_cmd()); execute_cmd() }; + let replacement = proc() { + try!(create_directory()); + try!(rustc_cmd()); + execute_cmd() + }; mem::replace(work, replacement); } } @@ -307,16 +337,98 @@ fn compile_custom_old(pkg: &Package, cmd: &str, }) } +// Contains the parsed output of a custom build script. +struct CustomBuildCommandOutput { + // paths to pass to rustc with the `-L` flag + library_paths: Vec, + // names and link kinds of libraries, suitable for the `-l` flag + library_links: Vec, + // metadata to pass to the immediate dependencies + metadata: Vec<(String, String)>, +} + +impl CustomBuildCommandOutput { + // Parses the output of a script. + // The `pkg_name` is used for error messages. + fn parse(mut input: B, pkg_name: &str) -> CargoResult { + let mut library_paths = Vec::new(); + let mut library_links = Vec::new(); + let mut metadata = Vec::new(); + + for line in input.lines() { + // unwrapping the IoResult + let line = try!(line.map_err(|e| human(format!("Error while reading\ + custom build output: {}", e)))); + + let mut iter = line.as_slice().splitn(1, |c: char| c == ':'); + if iter.next() != Some("cargo") { + // skip this line since it doesn't start with "cargo:" + continue; + } + let data = match iter.next() { + Some(val) => val, + None => continue + }; + + // getting the `key=value` part of the line + let mut iter = data.splitn(1, |c: char| c == '='); + let key = iter.next(); + let value = iter.next(); + let (key, value) = match (key, value) { + (Some(a), Some(b)) => (a, b), + // line started with `cargo:` but didn't match `key=value` + _ => return Err(human(format!("Wrong output for the custom\ + build script of `{}`:\n`{}`", pkg_name, line))) + }; + + if key == "rustc-flags" { + let mut flags_iter = value.split(|c: char| c == ' ' || c == '\t'); + loop { + let flag = match flags_iter.next() { + Some(f) => f, + None => break + }; + if flag != "-l" && flag != "-L" { + return Err(human(format!("Only `-l` and `-L` flags are allowed \ + in build script of `{}`:\n`{}`", + pkg_name, value))) + } + let value = match flags_iter.next() { + Some(v) => v, + None => return Err(human(format!("Flag in rustc-flags has no value\ + in build script of `{}`:\n`{}`", + pkg_name, value))) + }; + match flag { + "-l" => library_links.push(value.to_string()), + "-L" => library_paths.push(Path::new(value)), + + // was already checked above + _ => return Err(human("only -l and -L flags are allowed")) + }; + } + } else { + metadata.push((key.to_string(), value.to_string())) + } + } + + Ok(CustomBuildCommandOutput { + library_paths: library_paths, + library_links: library_links, + metadata: metadata, + }) + } +} + // Prepares a `Work` that executes the target as a custom build script. // `pkg` is the package the build script belongs to, and `root_pkg` is the package // Cargo is being run on. fn prepare_execute_custom_build(pkg: &Package, root_pkg: &Package, target: &Target, - cx: &mut Context) -> CargoResult { - // TODO: this shouldn't explicitly pass `KindTarget` for dest/deps_dir, we - // may be building a C lib for a plugin - let layout = cx.layout(pkg, KindTarget); - let output = layout.native(pkg); - let old_output = layout.proxy().old_native(pkg); + cx: &mut Context) + -> CargoResult { + let layout = cx.layout(pkg, KindForHost); + let script_output = layout.build(pkg); + let build_output = layout.build_out(pkg); // Building the command to execute let to_exec = try!(cx.target_filenames(target)); @@ -328,11 +440,12 @@ fn prepare_execute_custom_build(pkg: &Package, root_pkg: &Package, target: &Targ Some(cmd) => cmd, None => return Err(human(format!("failed to determine output of custom build script"))), }; - let to_exec = layout.root().join(to_exec); + let to_exec = script_output.join(to_exec); + // Filling environment variables let profile = target.get_profile(); let mut p = process(to_exec, pkg, cx) - .env("OUT_DIR", Some(&output)) + .env("OUT_DIR", Some(&build_output)) .env("CARGO_MANIFEST_DIR", Some(root_pkg.get_manifest_path() .display().to_string())) .env("NUM_JOBS", profile.get_codegen_units().map(|n| n.to_string())) @@ -354,25 +467,36 @@ fn prepare_execute_custom_build(pkg: &Package, root_pkg: &Package, target: &Targ None => {} } + // Building command let pkg = pkg.to_string(); + let work = proc() { + if !build_output.exists() { + try!(fs::mkdir(&build_output, USER_RWX) + .chain_error(|| { + internal("failed to create build output directory for build command") + })) + } - Ok(proc() { - // TODO: is this necessary? it's already part of layout::prepare - try!(if old_output.exists() { - fs::rename(&old_output, &output) - } else { - fs::mkdir(&output, USER_RWX) - }.chain_error(|| { - internal("failed to create output directory for build command") - })); - - try!(p.exec_with_output().map(|_| ()).map_err(|mut e| { + let output = try!(p.exec_with_output().map_err(|mut e| { e.msg = format!("Failed to run custom build command for `{}`\n{}", pkg, e.msg); e.mark_human() })); + + // parsing the output of the custom build script to check that it's correct + try!(CustomBuildCommandOutput::parse(BufReader::new(output.output.as_slice()), + pkg.as_slice())); + + // writing the output to the right directory + try!(fs::File::create(&script_output.join("output")).write(output.output.as_slice()) + .map_err(|e| { + human(format!("failed to write output of custom build command: {}", e)) + })); + Ok(()) - }) + }; + + Ok(work) } fn rustc(package: &Package, target: &Target, @@ -405,19 +529,19 @@ fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, let base = build_base_args(cx, base, package, target, crate_types.as_slice()); let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget); - let plugin_cmd = build_plugin_args(base, cx, package, target, KindPlugin); + let plugin_cmd = build_plugin_args(base, cx, package, target, KindForHost); let target_cmd = try!(build_deps_args(target_cmd, target, package, cx, KindTarget)); let plugin_cmd = try!(build_deps_args(plugin_cmd, target, package, cx, - KindPlugin)); + KindForHost)); Ok(match req { PlatformTarget => vec![(target_cmd, KindTarget)], - PlatformPlugin => vec![(plugin_cmd, KindPlugin)], + PlatformPlugin => vec![(plugin_cmd, KindForHost)], PlatformPluginAndTarget if cx.config.target().is_none() => vec![(target_cmd, KindTarget)], PlatformPluginAndTarget => vec![(target_cmd, KindTarget), - (plugin_cmd, KindPlugin)], + (plugin_cmd, KindForHost)], }) } @@ -548,12 +672,17 @@ fn build_base_args(cx: &Context, fn build_plugin_args(mut cmd: ProcessBuilder, cx: &Context, pkg: &Package, target: &Target, kind: Kind) -> ProcessBuilder { - cmd = cmd.arg("--out-dir"); - if target.is_example() { - cmd = cmd.arg(cx.layout(pkg, kind).examples()); + let out_dir = cx.layout(pkg, kind); + let out_dir = if target.get_profile().is_custom_build() { + out_dir.build(pkg) + } else if target.is_example() { + out_dir.examples().clone() } else { - cmd = cmd.arg(cx.layout(pkg, kind).root()); - } + out_dir.root().clone() + }; + + cmd = cmd.arg("--out-dir"); + cmd = cmd.arg(out_dir); let (_, dep_info_loc) = fingerprint::dep_info_loc(cx, pkg, target, kind); cmd = cmd.arg("--dep-info").arg(dep_info_loc); @@ -625,8 +754,8 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package, // plugin, then we want the plugin directory. Otherwise we want the // target directory (hence the || here). let layout = cx.layout(pkg, match kind { - KindPlugin => KindPlugin, - KindTarget if target.get_profile().is_for_host() => KindPlugin, + KindForHost => KindForHost, + KindTarget if target.get_profile().is_for_host() => KindForHost, KindTarget => KindTarget, }); @@ -647,7 +776,7 @@ pub fn process(cmd: T, pkg: &Package, cx: &Context) -> CargoResult { // When invoking a tool, we need the *host* deps directory in the dynamic // library search path for plugins and such which have dynamic dependencies. - let layout = cx.layout(pkg, KindPlugin); + let layout = cx.layout(pkg, KindForHost); let mut search_path = DynamicLibrary::search_path(); search_path.push(layout.deps().clone()); diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 77f43cac3..6f66b097a 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -2,7 +2,7 @@ pub use self::cargo_clean::{clean, CleanOptions}; pub use self::cargo_compile::{compile, compile_pkg, CompileOptions}; pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages}; pub use self::cargo_rustc::{compile_targets, Compilation, Layout, Kind, rustc_version}; -pub use self::cargo_rustc::{KindTarget, KindPlugin, Context, LayoutProxy}; +pub use self::cargo_rustc::{KindTarget, KindForHost, Context, LayoutProxy}; pub use self::cargo_rustc::{PlatformRequirement, PlatformTarget}; pub use self::cargo_rustc::{PlatformPlugin, PlatformPluginAndTarget}; pub use self::cargo_run::run; diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs index aab7e6556..97bcfd6de 100644 --- a/tests/test_cargo_compile_custom_build.rs +++ b/tests/test_cargo_compile_custom_build.rs @@ -20,10 +20,12 @@ test!(custom_build_compiled { .file("build.rs", r#" invalid rust file, should trigger a build error "#); + assert_that(p.cargo_process("build"), execs().with_status(101)); }) +/* test!(custom_build_script_failed { let p = project("foo") .file("Cargo.toml", r#" @@ -46,9 +48,10 @@ test!(custom_build_script_failed { execs().with_status(101) .with_stderr(format!("\ Failed to run custom build command for `foo v0.5.0 (file://{})` -Process didn't exit successfully: `{}` (status=101)", +Process didn't exit successfully: `{}` (status=101)", // TODO: TEST FAILS BECAUSE OF WRONG PATH p.root().display(), p.bin("build-script-build").display()))); }) +*/ test!(custom_build_env_vars { let p = project("foo") @@ -120,3 +123,31 @@ test!(custom_build_env_vars { assert_that(p.cargo_process("build").arg("--features").arg("bar_feat"), execs().with_status(0)); }) + +test!(custom_build_script_wrong_rustc_flags { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + build = "build.rs" + "#) + .file("src/main.rs", r#" + fn main() {} + "#) + .file("build.rs", r#" + fn main() { + println!("cargo:rustc-flags=-aaa -bbb"); + } + "#); + + assert_that(p.cargo_process("build"), + execs().with_status(101) + .with_stderr(format!("\ +Only `-l` and `-L` flags are allowed in build script of `foo v0.5.0 (file://{})`: +`-aaa -bbb +`", +p.root().display()))); +}) -- 2.30.2